Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Additional resources #232

Merged
merged 6 commits into from
Jun 20, 2018
Merged

Additional resources #232

merged 6 commits into from
Jun 20, 2018

Conversation

iherman
Copy link
Member

@iherman iherman commented Jun 18, 2018 •
edited by pr-preview bot
Loading

This PR implements

  • some decisions in Toronto, not yet in the document (cover and privacy policy as structural items)
  • adding the extra resources to the document, in case this gets accepted as proposed in yet another 'resource list' in the manifest? #225
  • adding cover, privacy policy, accessibility report, and external metadata to the manifest definition using various rel values.

If and when approved, this completes the manifest definition

Fix #225
Fix #216

Preview | Diff

iherman added 3 commits June 18, 2018 13:47
@iherman
Changes on the infoset section
294b806 
- moved (per resolutions in Toronto) the cover and the privacy policy to the structural infoset items
- added a new entry for the list of external resources
@iherman
Added the missing infoset mappings to the manifest:
6723101 
- list of external resources
- cover
- privacy policy
- accessibility report
- external metadata
@iherman
minor things
998b112
@HadrienGardeur
Copy link

Let's vote on this addition + the term that we should use during the call today.

TzviyaSiegman
TzviyaSiegman reviewed Jun 18, 2018
View reviewed changes
Copy link
Contributor

@TzviyaSiegman TzviyaSiegman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we dictate the means of providing text descriptions for the covers? Is that best practice? I would like to double check that practice. If we allow for more than one cover image, we should provide more information.
regarding new IANA defintions- Did we agree to define a new media-type? if we do that we also have to get UAs to recognize it. I don't think this is the easiest path forward for cover or accessibility report. Perhaps we should explore https://schema.org/Role.

@llemeurfr
Copy link
Contributor

I wonder why we should type extra resources with @type" : "PublicationLink".

And again, not convinced that the cover should be allowed as an extra resource (out of the boundaries of the publication) if it exists. I agree that it may be either in the reading order or in the f resource set.

@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

@TzviyaSiegman

Regarding new IANA defintions- Did we agree to define a new media-type? if we do that we also have to get UAs to recognize it. I don't think this is the easiest path forward for cover or accessibility report.

We do not need a separate media type for a cover, in my view, I guess the rel value should suffice in recognizing what the 'role' is for that thing.

Perhaps we should explore https://schema.org/Role.

Do you mean using Role as a replacement for our PublicationLink? That would mean using the properties defined in https://schema.org/Role, but I am not sure it is good enough. However, LinkRole (https://pending.schema.org/LinkRole) may do it (except that it is currently 'pending'). Looking at the current specification of PublicationLink, the properties url, name, description are the same; the rel would be linkRelationship (per definition it is the same as what we want). What is missing, however, is the usage of fileFormat (which is actually strange)....

(What happens if we do not use media types?)

@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

@llemeurfr

I wonder why we should type extra resources with @type" : "PublicationLink".

Well... in the Google checking tool it seems that at least the google implementation shouts at you if you do not provide an explicit type for all objects. That is why I have put it into all examples...

...not convinced that the cover should be allowed as an extra resource (out of the boundaries of the publication) if it exists.

I am flexible on that one (the PR is definitely not final, depends on these discussions.)

@TzviyaSiegman
Copy link
Contributor

@iherman LinkRole (https://pending.schema.org/LinkRole) instead of PublicationLink looks like it's worth exploring. We should find out if it's pending because it is incomplete of just waiting for the next push.

@HadrienGardeur
Copy link

What is missing, however, is the usage of fileFormat (which is actually strange)....

(What happens if we do not use media types?)

I recently discovered that fileFormat is superceded by encodingFormat: https://schema.org/encodingFormat

I'll quote that page:

Media type typically expressed using a MIME format (see IANA site and MDN reference) e.g. application/zip for a SoftwareApplication binary, audio/mpeg for .mp3 etc.).
...
Unregistered or niche encoding and file formats can be indicated instead via the most appropriate URL, e.g. defining Web page or a Wikipedia/Wikidata entry.

Once again, I'm really not a big fan of the schema.org naming conventions.

@HadrienGardeur
Copy link

Well... in the Google checking tool it seems that at least the google implementation shouts at you if you do not provide an explicit type for all objects. That is why I have put it into all examples...

This can be handled in our context document, we don't need to include a type for each element listed in readingOrder, resources or links.

@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

@HadrienGardeur

What is missing, however, is the usage of fileFormat (which is actually strange)....

(What happens if we do not use media types?)

I recently discovered that fileFormat is superceded by encodingFormat: https://schema.org/encodingFormat

Yep, I discovered it today. But it says that it is a property on CreativeWork. If this was RDF domain, that would have consequences, I am not sure what it means in schema.org land if I use it on a type that is not supposed to hold it...

@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

To answer my own question above: the Google tool does complain if fileFormat is used on a different type than CreativeWork...

@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

I propose what we do is to add an editorial note at the definition of our own PublicationList that we explore the usage of LinkRole, but we have to have a discussion with schema.org on the usage of fileFormat (or encodingFormat). Add it to our list to discuss...

@iherman iherman mentioned this pull request Jun 18, 2018
Closed
@iherman
Copy link
Member Author

iherman commented Jun 18, 2018

Ref: schemaorg/schemaorg#1959

@iherman iherman mentioned this pull request Jun 19, 2018
Closed
GarthConboy
GarthConboy approved these changes Jun 19, 2018
View reviewed changes
Copy link
Contributor

@GarthConboy GarthConboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good step forward and we should commit and tune from here.

The notion that all these extraResources are beyond the bounds of the publication may want to be a "may be" – might an RS want to find the cover here, even said happens to be used within an HTML file within the default reading order?

@iherman
Copy link
Member Author

iherman commented Jun 20, 2018

@GarthConboy I think there should be normative statement somewhere that the three lists (reading order, resources, and extra resources) should be mutually disjoint. I guess there are some statement in the current PR that could be put in to reinforce this.

Should we open a separate issue on this?

iherman added 2 commits June 20, 2018 09:01
@iherman
Added a note on reorganization, and removed the (erroneous) remark wh…
a870cc8 
…ereby cover, a11y report, etc, should only be in the (external) resource list.
@iherman
Updated the json-ld and example files.
fe4e3a4
@GarthConboy GarthConboy merged commit 8dd6539 into master Jun 20, 2018
@iherman iherman mentioned this pull request Jun 20, 2018
Closed
@iherman iherman mentioned this pull request Jun 20, 2018
Closed
@iherman iherman deleted the additional-resources branch June 20, 2018 15:57
@iherman iherman mentioned this pull request Jun 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TzviyaSiegman TzviyaSiegman TzviyaSiegman left review comments

@GarthConboy GarthConboy GarthConboy approved these changes

@HadrienGardeur HadrienGardeur Awaiting requested review from HadrienGardeur

@mattgarrish mattgarrish Awaiting requested review from mattgarrish

@llemeurfr llemeurfr Awaiting requested review from llemeurfr

@laudrain laudrain Awaiting requested review from laudrain

@clapierre clapierre Awaiting requested review from clapierre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iherman @HadrienGardeur @llemeurfr @TzviyaSiegman @GarthConboy